⚠️ Drop unnecessary fields from machine status.nodeRef #12352
Conversation
k8s-ci-robot
added
area/machine
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/L
|
/test pull-cluster-api-e2e-conformance-ci-latest-main |
| @@ -556,6 +556,15 @@ type MachineStatus struct { | |||
| Deprecated *MachineDeprecatedStatus `json:"deprecated,omitempty"` | |||
| } | |||
|
|
|||
| // MachineNodeReference is a reference to a the node running on the machine. | |||
There was a problem hiding this comment.
| // MachineNodeReference is a reference to a the node running on the machine. | |
| // MachineNodeReference is a reference to the node running on the machine. |
| @@ -199,6 +200,8 @@ proposal because most of the changes described below are a consequence of the wo | |||
| - Support for terminal errors has been dropped. | |||
| - `status.failureReason` and `status.failureMessage` will continue to exist temporarily under `status.deprecated.v1beta1`. | |||
| - The const values for `Failed` phase has been deprecated in the enum type for `status.phase` (controllers are not setting this value anymore) | |||
| - The type of the `status.nodeRef` field has been changed from `corev1.ObjectReference` to `MachineNodeReference`. | |||
| - The following fields has been removed from `status.nodeRef`: `kind`, `namespace`, `uid`, `apiVersion`, `resourceVersion`, `fieldPath` | |||
There was a problem hiding this comment.
| - The following fields has been removed from `status.nodeRef`: `kind`, `namespace`, `uid`, `apiVersion`, `resourceVersion`, `fieldPath` | |
| - The following fields have been removed from `status.nodeRef`: `kind`, `namespace`, `uid`, `apiVersion`, `resourceVersion`, `fieldPath` |
| Kind: "Node", | ||
| Namespace: "test-node", | ||
| }, | ||
| NodeRef: &clusterv1.MachineNodeReference{}, |
There was a problem hiding this comment.
Let's add Name: "test-node". I think that was the intention of the old code
(same above in the other case)
| @@ -68,7 +68,7 @@ controller_runtime_reconcile_panics_total{controller="clusterclass"} 0 | |||
| # TYPE controller_runtime_webhook_panics_total counter | |||
| controller_runtime_webhook_panics_total 0 | |||
| `), | |||
| wantErr: "panic occurred in \"cluster\" controller", | |||
| wantErr: "1 panics occurred in \"cluster\" controller (check logs for more details)", | |||
There was a problem hiding this comment.
Are these tests not executed in CI?
(If yes, let's please fix that or open a help wanted issue for it. Doesn't have to be part of this PR of course)
They were not green before this change, right?
|
/assign @JoelSpeed |
| // +required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=253 | ||
| Name string `json:"name"` |
There was a problem hiding this comment.
Names are DNS1123 subdomain validated.
Lets borrow this comment
What this PR does / why we need it:
Another cleanup to keep our API surface as small as clean as we can
Part of #6539
/area machine